inifile: Fix non-reentrant Find() returning pointer to static buffer#3763
Merged
andypugh merged 2 commits intoLinuxCNC:masterfrom Feb 1, 2026
Merged
inifile: Fix non-reentrant Find() returning pointer to static buffer#3763andypugh merged 2 commits intoLinuxCNC:masterfrom
andypugh merged 2 commits intoLinuxCNC:masterfrom
Conversation
22a9237 to
9a63d9c
Compare
Contributor
|
Why not fix the C API at the same time? It isn't used many places. |
Author
|
Fixed the C API as well, both C and C++ APIs should now be reentrant/thread-safe. |
Contributor
|
To be pedantic... |
Author
|
No problem, testing some stuff on the 9d branch, will add deprecated ASAP. |
3d2f829 to
c7d588d
Compare
Author
|
Done, I just forced pushed, so it stays in two relatively clean commits, attribute((deprecated)) more in line with the codebase. Devs should get a warning on compilation if they try to use it |
added 2 commits
February 1, 2026 08:04
IniFile::Find() used a static buffer to return results, causing bugs when multiple Find() calls were made before using the first result. Change Find() to return std::optional<std::string> instead of std::optional<const char*>. Each caller now owns their result copy. Core changes: - Remove static buffer from Find(), return std::string by value - Update all callers to use s->c_str() where const char* is needed - Use string methods (.empty(), .length(), ==) instead of C functions C API compatibility maintained via static storage in iniFind() wrapper. No functional changes - all updates are behavior-preserving.
Follow-up to the C++ IniFile::Find() reentrancy fix. The previous commit changed Find() to return std::string by value, but the C API wrapper iniFind() still used static storage, making it non-reentrant. Add iniFindString() that takes a caller-provided buffer, making it safe for concurrent use and multiple sequential calls. Reimplement iniFind() as a deprecated wrapper around iniFindString(). Migrate all C callers to iniFindString(): - mb2hal: 11 call sites - vfdb_vfd: 2 call sites - vfs11_vfd: 4 call sites - halcmd: 2 call sites Also clean up dead commented-out code in mb2hal and halrmt.
c7d588d to
3575974
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
IniFile::Find()returned a pointer to a static buffer, causing reentrancy bugs when multipleFind()calls were made before using the first result:The code had a comment acknowledging this issue:
Solution
Changed
IniFile::Find()to returnstd::optional<std::string>instead ofstd::optional<const char*>. Each caller now owns their own copy of the result.Before
After
Changes Made
Core API Changes
inifile.hh: ChangedFind()signature to returnstd::optional<std::string>inifile.cc: Removed static buffer, returnstd::stringby valueemcIniFile.hh: Updated wrapper class to match new return typeCaller Updates (20 files)
All callers updated to use the new API:
s->c_str()when passing to C APIs requiringconst char**s == "value"instead ofstrcmp()for string comparisonss->find_first_of()instead ofstrchr()for character searchess->length()instead ofstrlen()s->empty()instead of checkings[0]Migration Guide
String Output
String Comparison
Character Search
C API Compatibility
The legacy C function
iniFind()maintains backward compatibility but still uses static storage:Important: C code calling
iniFind()must still copy the result immediately if it needs to be preserved. This preserves the original C API behavior for backward compatibility.Benefits
Build Status
✅ Compiles successfully with no warnings or errors